Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix for not receiving notification #11575

Closed
wants to merge 1 commit into from

Conversation

sovainfo
Copy link
Contributor

@sovainfo sovainfo commented Aug 12, 2016

Summary of Changes

Changing condition from GT to GE so it detects the update and sends the mail

Testing Instructions

Apply fix and check receiving the notification

@zero-24
Copy link
Contributor

zero-24 commented Aug 16, 2016

That can't work as you get out of the party here: https://github.com/joomla/joomla-cms/pull/11575/files#diff-389d7f2bc20fb10dd7192b69423c7ba6R51

$updater->findUpdates(array($eid), $cache_timeout, JUpdater::STABILITY_STABLE, true);

This change is not needed as JUpdater::STABILITY_STABLE is default
and the last true means that you got it for reinstall.

https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/updater/updater.php#L115

public function findUpdates($eid = 0, $cacheTimeout = 0, $minimum_stability = self::STABILITY_STABLE, $includeCurrent = false)

@sovainfo
Copy link
Contributor Author

No, it is about the 4th parameter. It needs to be true to change the condition GT into GE.
And it does work! Why would it not work for notification when it does for Joomla! Update !!!

@zero-24
Copy link
Contributor

zero-24 commented Aug 16, 2016

Please double check this line:
https://github.com/joomla/joomla-cms/blob/staging/plugins/system/updatenotification/updatenotification.php#L51

This can never be true with $cache_timeout == 0 ;) So you will never reache the line you changed with cachetimeout == 0

The 4th paramenter is for the reinstall feature and needs to be in com_joomlaupdate to prodvide the reinstall function. Here it is not correct as if it is true it allways return a update object and if you are on the last version you get the current version back which is not correct in this case here.

@sovainfo
Copy link
Contributor Author

This is not about $cache_timeout = 0 ! @wilsonge already explained why that doesn't work as expected.
It is about not receiving those emails while on 1 to 6.

@zero-24
Copy link
Contributor

zero-24 commented Aug 16, 2016

It is about not receiving those emails while on 1 to 6.

Yes and that works great on all sites i manage without any change 😄 And i don't know anyone who have issues with it nor i can reproduce issues with it. 😄

@sovainfo
Copy link
Contributor Author

It didn't on mine, which is the reason for debugging it. On the forum there are also reports of not receiving them. I traced it back to this situation and the solution works.
But if there is no interest in fixing it, fine with me. Saves me disabling the plugin!

@zero-24
Copy link
Contributor

zero-24 commented Aug 16, 2016

@sovainfo let this be double checked by something 3rd e.g. @wilsonge or @mbabker

I think it is wrong with this change applyed but I'm happy to hear something different if I'm wrong. Thanks

@sovainfo
Copy link
Contributor Author

Currently debugging my third clean install. Received the first mail. Changed timeout from 6 to 1 and from hours to minutes. Don't receive any furter mails.

@zero-24
Copy link
Contributor

zero-24 commented Aug 16, 2016

Where did you change the value from hours to minutes? In code where? Did you clear the update cache and the last check time?

Can you give me a step by step way to reproduce the problem?

@sovainfo
Copy link
Contributor Author

sovainfo commented Aug 16, 2016

Change hours to minutes: line 44 of plugin:

$cache_timeout = 60 * $cache_timeout;

Step by step way to reproduce: refresh after a minute. used the control panel in the backend. But, as you know with this plugin it can be any screen, frontend or backend doesn't matter!

@zero-24
Copy link
Contributor

zero-24 commented Aug 16, 2016

Just frontend. I will double check that in the frontend tomorrow.

@sovainfo
Copy link
Contributor Author

No, it is not just frontend!

@zero-24
Copy link
Contributor

zero-24 commented Aug 16, 2016

Hmm correct my memory fouls me. Let me double check that tomorrow.

@sovainfo
Copy link
Contributor Author

Sorry for you, but larded both installer and notification with JLog::add statements to find the cause. Found out it was comparing 3.6.2 with 3.6.2.

@zero-24
Copy link
Contributor

zero-24 commented Aug 16, 2016

Ok so you agree with me that we don't need this change?

@sovainfo
Copy link
Contributor Author

No, comparing 3.6.2 with 3.6.2 on a 3.6.0 site results in NO mail. When 4th parameter TRUE you get a mail. So, of course this is needed. Wouldn't spent so much time here, when not needed.

@zero-24
Copy link
Contributor

zero-24 commented Aug 17, 2016

No, comparing 3.6.2 with 3.6.2 on a 3.6.0 site results in NO mail.

Where it compare 3.6.2 and 3.6.2 on a 3.6.0 webseite? That is wrong but if Joomla thinks you are on the last version you should never get a mail!

Please post your system information page and doubel check the Database version (Extensions -> manage -> database)

I have tested the current plugin successful please follow this steps and let me know what you did different:

  • install 3.6.1
  • login to the backend or
  • navigate to the frontend
  • you get the mail

The other way arround

  • there is a installed 3.6.2 we want to test
  • go to /libraries/cms/version/version.php
  • change the line 41 to const DEV_LEVEL = '1';
  • go to the database fixer and hit fix (that fixes the joomla version back to 3.6.1 in the databse to be sure)
  • double check that the plugin updatenotification is enabled
  • go to /plugins/system/updatenotification/updatenotification.php
  • add to the line 50 $last = 0; (this way you pass the last run check without hacking the database) (https://github.com/joomla/joomla-cms/blob/3.6.2/plugins/system/updatenotification/updatenotification.php#L50)
  • go to the Backend Extensions -> manage -> Update -> clear cache (this is required as there is also the cache timeout)
  • hit the frontend (or backend) with F5
  • you get the mail

Please test this without any code change.

The other test (with this patch applyed) results in the following:

Please double check your steps and let me know what happen. I still see NO error in the current code as it works great 😄

@sovainfo
Copy link
Contributor Author

Where it compare 3.6.2 and 3.6.2 on a 3.6.0 webseite?

line 359 of libraries/joomla/updater/updater.php
if (version_compare($current_update->version, $update->version, $operator) == 1)
Contrary to what the documentation line above it claims it checks whether the update found is more recent than the one cached in #__updates.

@zero-24
Copy link
Contributor

zero-24 commented Aug 18, 2016

Yes that is correct. As stated above please clear the cached updates. Bevor testing the frontend

@sovainfo
Copy link
Contributor Author

sovainfo commented Aug 18, 2016

Yes that is correct.

What do you mean?

As stated above please clear the cached updates. Bevor testing the frontend

Why do you say this?

@zero-24
Copy link
Contributor

zero-24 commented Aug 18, 2016

There is no need to save the new update in the database if it exists.

You need to clear the update cache bevor the update gets checked. There is also a cache time that take into account.

@sovainfo
Copy link
Contributor Author

sovainfo commented Aug 18, 2016

There is no need to save the new update in the database if it exists.

Sounds like you don't understand the functionality. This is not about saving update information. This is about determining whether there is a new update to be notified about.

You need to clear the update cache bevor the update gets checked. There is also a cache time that take into account.

That is incorrect! The whole purpose of this check is to see whether in mean time there have been new updates. So, on a J350 with cached info about J351, it overrules it with J360 because that is the most recent. At least for J350 because the update server is set up to go through J360 when going to J362.
So, refreshing cache is not a requirement!

@Kev1n337
Copy link
Contributor

I have tested this item 🔴 unsuccessfully on bc57d35

@icampus

I installed an older joomla-version (3.7.4). Without applying the patch I received the update notification via email, that 3.7.5 is available.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11575.

@roland-d
Copy link
Contributor

@sovainfo Can you check if the issue has been resolved in the meantime for you?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11575.

@sovainfo
Copy link
Contributor Author

Lost interest in Joomla.

@sovainfo sovainfo closed this Aug 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants